Skip to content

Mitigated Serial Monitor resource exhaustion #2491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 29, 2014

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Dec 10, 2014

Fixes #2233

Tested on a Dual-core 1.8Ghz Linux 64bit system with 4GB of RAM: the Serial Monitor performance are dramatically improved.

/cc @PaulStoffregen @willie68 @xxxajk @ffissore

@cmaglie cmaglie self-assigned this Dec 10, 2014
@cmaglie cmaglie added this to the Release 1.0.7 milestone Dec 10, 2014
@cmaglie cmaglie added Component: IDE user interface The Arduino IDE's user interface feature request A request to make an enhancement (not a bug fix) Version: 1.0.x labels Dec 10, 2014
len = getDocument().getLength();
if (len > maxChars) {
int n = len - maxChars;
System.out.println("trimDocument: remove " + n + " chars");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left over

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@PaulStoffregen
Copy link
Contributor

Oh, opps, yeah, I missed that print before committing the code.

@ffissore
Copy link
Contributor

It looks like if I unckeck autoscrolling, it stops working. Right after re-enabling it, it says (with that removed println) it has removed all the accumulated extra chars. Something like

trimDocument: remove 200000 chars
... disable autoscroll and wait a couple of minutes, then reenable it ...
trimDocument: remove 8000000 chars

@PaulStoffregen
Copy link
Contributor

Yup, work remains to be done for turning off autoscroll. I wrote a more detailed explanation here:

#2233 (comment)

@PaulStoffregen
Copy link
Contributor

If you or anyone else reading works on this, please understand that you absolutely must not trim the document while autoscroll is turned off. Doing so ruins the user experience. But you can discard incoming data (before it's added to the document), hopefully after some reasonable threshold that won't be hit for slower data rates and only brief disable of autoscroll.

Before this patch every byte received from Serial
invokes a String allocation, not really efficient.

Moreover a InputStreamReader is chained on the serial
InputStream to correctly convert bytes into UTF-8
characters.
When the "autoscroll" checkbox is deselected the buffer may continue
to grow up to twice of the maximum size.

This is a compromise to ensure a better user experience and, at the
same time, reduce the chance to lose data and get "holes" in the
serial stream.

See arduino#2491
@cmaglie
Copy link
Member Author

cmaglie commented Dec 23, 2014

I've updated the pull request:

  • I've found (and fixed) the performance killer here: cmaglie@63f5d26#diff-4ed4a5bd21c427b12e6ebdb4f9f57e4aL273
    as you can see a new string was allocated for every character received. This has (incidentally?) already been fixed in 1.6.0 when we moved to JSSC. Now the CPU usage dropped from 100 to 20!
  • The autoscroll limit has been implemented as suggested by @PaulStoffregen.

@cmaglie cmaglie merged commit 8e0a311 into arduino:master Dec 29, 2014
@cmaglie cmaglie deleted the serial-monitor-oom branch December 29, 2014 15:37
@per1234 per1234 added Component: IDE Serial monitor Tools > Serial Monitor and removed Component: IDE user interface The Arduino IDE's user interface labels Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: IDE Serial monitor Tools > Serial Monitor feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 min with serial monitor results in OutOfMemoryException
4 participants